Skip to content

Fix NoMethodError when email domain is nil#53

Open
tnantoka wants to merge 1 commit into
synth:mainfrom
tnantoka:fix/domain-verifier-nil-email
Open

Fix NoMethodError when email domain is nil#53
tnantoka wants to merge 1 commit into
synth:mainfrom
tnantoka:fix/domain-verifier-nil-email

Conversation

@tnantoka

Copy link
Copy Markdown

Hi, and thanks for this great gem. I've been using it happily for years.

After the change in #42, some of my accounts started getting a NoMethodError
(undefined method 'casecmp?' for nil) on login. It turned out that some
Microsoft accounts (personal ones, in my case) don't return a mail, so
email_domain becomes nil and casecmp? raises before the other checks are
evaluated, even with skip_domain_verification set.

I fixed it with safe navigation, so a missing email simply does not match by
domain and falls through to the existing skip / JWT claim / error paths.

I saw #17 (comment) and I understand this gem isn't really aimed at personal accounts,
but since this change has no real downside (org accounts behave exactly the
same), I would be glad if you could consider merging it.

Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant